-
Notifications
You must be signed in to change notification settings - Fork 5
Initial upgrade to the API v0.17.x #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This is in draft state because tests are still missing, but I don't want to invest a lot of time in tests yet in case some major changes come up from the review. The basics are tested against the sandbox and it works, so the code itself should be pretty much ready for a serious review. |
5c8e3f4 to
764c561
Compare
|
Updated with a second iteration with some fixes after the integration with the SDK was completed (in the sense that all SDK tests are passing). Still missing tests, which should be the next step, but I'm moderately confident the PR should be very close to being final (implementation-wise) because it is already passing all SDK tests. |
f26e062 to
efcf9e2
Compare
|
To make review less of a torture, I decided it is probably better to split this in at least one PR per new grpc call. This PR also includes the base work for starting the migration, but I consider it finished. I only keep it as a draft because it depends on #144. |
aa0a3d5 to
2758479
Compare
| if longitude is None: | ||
| issues.append("longitude out of range [-180, 180]") | ||
|
|
||
| country_code = message.country_code or None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is where the conversion from "" to None happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment about logging, rest are optional.
| from ._microgrid_info_proto import microgrid_info_from_proto | ||
| from .id import ComponentId | ||
|
|
||
| DEFAULT_GRPC_CALL_TIMEOUT = 60.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a lot for a client that can only be accessed from a local network. Was it always like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you mean the 60 seconds timeout, yes it was like this before too.
|
|
||
|
|
||
| @pytest.fixture | ||
| async def client() -> AsyncIterator[MicrogridApiClient]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it have to be async?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because of the async with client: inside.
|
Updated @shsms |
The API changed so much in v0.17 that building on top of the previous version will just add noise to the diff of the following commits, and we won't have checks passing for a long time either if we go this route because after upgrading the API bindings files nothings works anymore. Signed-off-by: Leandro Lucarella <[email protected]>
|
I will need to adapt this to the latest changes in v0.x.x (using common IDs from |
We also need to bump many other dependencies to be compatible with the new `frequenz-api-microgrid` version. Signed-off-by: Leandro Lucarella <[email protected]>
This will be used to retrieve the microgrid information. Signed-off-by: Leandro Lucarella <[email protected]>
This will be used to retrieve the microgrid information. Signed-off-by: Leandro Lucarella <[email protected]>
This will be used to retrieve the microgrid information. Signed-off-by: Leandro Lucarella <[email protected]>
`DEFAULT_GRPC_CALL_TIMEOUT` could be potentially tuned by the user and `DEFAULT_CHANNEL_OPTIONS` is important as documentation for the defaults. Signed-off-by: Leandro Lucarella <[email protected]>
This commit introduces a new module which provides a framework for testing gRPC client implementations. The module simplifies mocking gRPC stub interactions and asserting client behavior for both unary and streaming calls. It allows defining test cases in separate Python files with a conventional structure, making tests more organized and maintainable. Key features include: - Discovery of test case files based on directory structure. - Mocking of gRPC responses and errors. - Assertion helpers for gRPC requests and client results/exceptions. - Support for unary-unary and unary-stream gRPC methods. Detailed usage instructions and examples are available in the module's docstring. Please note this module is added here temporarily and will be moved to the `frequenz-client-base` repository as a general test utility in the near future. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
We call the `MicrogridApiClient` method `get_microgrid_info()` to be more forward-compatible with upcoming changes. Signed-off-by: Leandro Lucarella <[email protected]>
|
Rebased. Changes:
Diffdiff --git a/pyproject.toml b/pyproject.toml
index f187789..e483582 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -3,9 +3,9 @@
[build-system]
requires = [
- "setuptools == 80.3.1",
+ "setuptools == 80.9.0",
"setuptools_scm[toml] == 8.3.1",
- "frequenz-repo-config[lib] == 0.13.3",
+ "frequenz-repo-config[lib] == 0.13.4",
]
build-backend = "setuptools.build_meta"
@@ -36,13 +36,14 @@ classifiers = [
]
requires-python = ">= 3.11, < 4"
dependencies = [
- "frequenz-api-microgrid >= 0.17.1, < 0.18.0",
- "frequenz-channels >= 1.0.0-rc1, < 2.0.0",
- "frequenz-client-base >= 0.8.0, < 0.12.0",
- "grpcio >= 1.59.0, < 2",
- "protobuf >= 4.25.3, < 6",
+ "frequenz-api-microgrid >= 0.17.2, < 0.18.0",
+ "frequenz-channels >= 1.6.1, < 2.0.0",
+ "frequenz-client-base >= 0.10.0, < 0.12.0",
+ "frequenz-client-common >= 0.3.2, < 0.4.0",
+ "grpcio >= 1.72.1, < 2",
+ "protobuf >= 6.31.1, < 7",
"timezonefinder >= 6.2.0, < 7",
- "typing-extensions >= 4.6.0, < 5",
+ "typing-extensions >= 4.13.0, < 5",
]
dynamic = ["version"]
@@ -66,29 +67,29 @@ dev-mkdocs = [
"mkdocs-gen-files == 0.5.0",
"mkdocs-literate-nav == 0.6.2",
"mkdocs-macros-plugin == 1.3.7",
- "mkdocs-material == 9.6.12",
+ "mkdocs-material == 9.6.14",
"mkdocstrings[python] == 0.29.1",
- "mkdocstrings-python == 1.16.10",
- "frequenz-repo-config[lib] == 0.13.3",
+ "mkdocstrings-python == 1.16.11",
+ "frequenz-repo-config[lib] == 0.13.4",
]
dev-mypy = [
- "mypy == 1.15.0",
+ "mypy == 1.16.0",
"grpc-stubs == 1.53.0.6",
"types-Markdown == 3.8.0.20250415",
- "types-protobuf == 5.29.1.20250403",
+ "types-protobuf == 6.30.2.20250516",
# For checking the noxfile, docs/ script, and tests
"frequenz-client-microgrid[dev-mkdocs,dev-noxfile,dev-pytest]",
]
-dev-noxfile = ["nox == 2025.5.1", "frequenz-repo-config[lib] == 0.13.3"]
+dev-noxfile = ["nox == 2025.5.1", "frequenz-repo-config[lib] == 0.13.4"]
dev-pylint = [
- "pylint == 3.3.6",
+ "pylint == 3.3.7",
# For checking the noxfile, docs/ script, and tests
"frequenz-client-microgrid[dev-mkdocs,dev-noxfile,dev-pytest]",
]
dev-pytest = [
"pytest == 8.3.5",
- "frequenz-repo-config[extra-lint-examples] == 0.13.3",
- "pytest-mock == 3.14.0",
+ "frequenz-repo-config[extra-lint-examples] == 0.13.4",
+ "pytest-mock == 3.14.1",
"pytest-asyncio == 0.26.0",
"async-solipsism == 0.7",
]
@@ -161,7 +162,15 @@ disable = [
]
[tool.pytest.ini_options]
-addopts = "-W=all -Werror -Wdefault::DeprecationWarning -Wdefault::PendingDeprecationWarning -vv"
+addopts = "-vv"
+filterwarnings = [
+ "error",
+ "once::DeprecationWarning",
+ "once::PendingDeprecationWarning",
+ # We use a raw string (single quote) to avoid the need to escape special
+ # chars as this is a regex
+ 'ignore:Protobuf gencode version .*exactly one major version older.*:UserWarning',
+]
testpaths = ["tests", "src"]
asyncio_mode = "auto"
asyncio_default_fixture_loop_scope = "function"
diff --git a/src/frequenz/client/microgrid/_client.py b/src/frequenz/client/microgrid/_client.py
index 7331e36..7ba65b6 100644
--- a/src/frequenz/client/microgrid/_client.py
+++ b/src/frequenz/client/microgrid/_client.py
@@ -12,13 +12,13 @@
from frequenz.api.microgrid.v1 import microgrid_pb2_grpc
from frequenz.client.base import channel, client, retry, streaming
+from frequenz.client.common.microgrid.components import ComponentId
from google.protobuf.empty_pb2 import Empty
from typing_extensions import override
from ._exception import ClientNotConnected
from ._microgrid_info import MicrogridInfo
from ._microgrid_info_proto import microgrid_info_from_proto
-from .id import ComponentId
DEFAULT_GRPC_CALL_TIMEOUT = 60.0
"""The default timeout for gRPC calls made by this client (in seconds)."""
@@ -129,10 +129,11 @@ def stub
async def get_microgrid_info( # noqa: DOC502 (raises ApiClientError indirectly)
self,
) -> MicrogridInfo:
- """Fetch the information about the local microgrid.
+ """Retrieve information about the local microgrid.
- This consists of information that describes the overall microgrid, as opposed to
- its electrical components or sensors, e.g., the microgrid ID, location.
+ This consists of information about the overall microgrid, for example, the
+ microgrid ID and its location. It does not include information about the
+ electrical components or sensors in the microgrid.
Returns:
The information about the local microgrid.
@@ -142,7 +143,6 @@ def stub
most likely a subclass of
[GrpcError][frequenz.client.microgrid.GrpcError].
"""
- print(f"CALLED get_microgrid_info(): {self.stub.GetMicrogridMetadata=}")
microgrid = await client.call_stub_method(
self,
lambda: self.stub.GetMicrogridMetadata(
diff --git a/src/frequenz/client/microgrid/_microgrid_info.py b/src/frequenz/client/microgrid/_microgrid_info.py
index e73c421..b1f65c9 100644
--- a/src/frequenz/client/microgrid/_microgrid_info.py
+++ b/src/frequenz/client/microgrid/_microgrid_info.py
@@ -10,10 +10,10 @@
from functools import cached_property
from frequenz.api.common.v1.microgrid import microgrid_pb2
+from frequenz.client.common.microgrid import EnterpriseId, MicrogridId
from ._delivery_area import DeliveryArea
from ._location import Location
-from .id import EnterpriseId, MicrogridId
_logger = logging.getLogger(__name__)
diff --git a/src/frequenz/client/microgrid/_microgrid_info_proto.py b/src/frequenz/client/microgrid/_microgrid_info_proto.py
index 53211b3..0640cde 100644
--- a/src/frequenz/client/microgrid/_microgrid_info_proto.py
+++ b/src/frequenz/client/microgrid/_microgrid_info_proto.py
@@ -8,6 +8,7 @@
from frequenz.api.common.v1.microgrid import microgrid_pb2
from frequenz.client.base import conversion
+from frequenz.client.common.microgrid import EnterpriseId, MicrogridId
from ._delivery_area import DeliveryArea
from ._delivery_area_proto import delivery_area_from_proto
@@ -15,7 +16,6 @@
from ._location_proto import location_from_proto
from ._microgrid_info import MicrogridInfo, MicrogridStatus
from ._util import enum_from_proto
-from .id import EnterpriseId, MicrogridId
_logger = logging.getLogger(__name__)
diff --git a/src/frequenz/client/microgrid/id.py b/src/frequenz/client/microgrid/id.py
deleted file mode 100644
index cfa2aa6..0000000
--- a/src/frequenz/client/microgrid/id.py
+++ /dev/null
@@ -1,223 +0,0 @@
-# License: MIT
-# Copyright © 2025 Frequenz Energy-as-a-Service GmbH
-
-r'''Provides strongly-typed unique identifiers for entities.
-
-This module offers a base class,
-[`BaseId`][frequenz.client.microgrid.id.BaseId], which can be subclassed to
-create distinct ID types for different components or concepts within a system.
-These IDs ensure type safety, meaning that an ID for one type of entity (e.g., a
-sensor) cannot be mistakenly used where an ID for another type (e.g., a
-microgrid) is expected.
-
-# Creating Custom ID Types
-
-To define a new ID type, create a class that inherits from
-[`BaseId`][frequenz.client.microgrid.id.BaseId] and provide a unique
-`str_prefix` as a keyword argument in the class definition. This prefix is used
-in the string representation of the ID and must be unique across all ID types.
-
-Note:
- The `str_prefix` must be unique across all ID types. If you try to use a
- prefix that is already registered, a `ValueError` will be raised when defining
- the class.
-
-To encourage consistency, the class name must end with the suffix "Id" (e.g.,
-`MyNewId`). This check can be bypassed by passing `allow_custom_name=True` when
-defining the class (e.g., `class MyCustomName(BaseId, str_prefix="MCN",
-allow_custom_name=True):`).
-
-Tip:
- Use the [`@typing.final`][typing.final] decorator to prevent subclassing of
- ID classes.
-
-Example: Creating a standard ID type
- ```python
- from typing import final
- from frequenz.client.microgrid.id import BaseId
-
- @final
- class InverterId(BaseId, str_prefix="INV"):
- """A unique identifier for an inverter."""
-
- inv_id = InverterId(123)
- print(inv_id) # Output: INV123
- print(int(inv_id)) # Output: 123
- ```
-
-Example: Creating an ID type with a non-standard name
- ```python
- from typing import final
- from frequenz.client.microgrid.id import BaseId
-
- @final
- class CustomNameForId(BaseId, str_prefix="CST", allow_custom_name=True):
- """An ID with a custom name, not ending in 'Id'."""
-
- custom_id = CustomNameForId(456)
- print(custom_id) # Output: CST456
- print(int(custom_id)) # Output: 456
- ```
-
-# Predefined ID Types
-
-This module predefines the following ID types:
-
-- [`ComponentId`][frequenz.client.microgrid.id.ComponentId]: For identifying
- generic components.
-- [`MicrogridId`][frequenz.client.microgrid.id.MicrogridId]: For identifying
- microgrids.
-- [`SensorId`][frequenz.client.microgrid.id.SensorId]: For identifying sensors.
-'''
-
-
-from typing import Any, ClassVar, Self, cast, final
-
-
-class BaseId:
- """A base class for unique identifiers.
-
- Subclasses must provide a unique `str_prefix` keyword argument during
- definition, which is used in the string representation of the ID.
-
- By default, subclass names must end with "Id". This can be overridden by
- passing `allow_custom_name=True` during class definition.
-
- For more information and examples, see the [module's
- documentation][frequenz.client.microgrid.id].
- """
-
- _id: int
- _str_prefix: ClassVar[str]
- _registered_prefixes: ClassVar[set[str]] = set()
-
- def __new__(cls, *_: Any, **__: Any) -> Self:
- """Create a new instance of the ID class, only if it is a subclass of BaseId."""
- if cls is BaseId:
- raise TypeError("BaseId cannot be instantiated directly. Use a subclass.")
- return super().__new__(cls)
-
- def __init_subclass__(
- cls,
- *,
- str_prefix: str,
- allow_custom_name: bool = False,
- **kwargs: Any,
- ) -> None:
- """Initialize a subclass, set its string prefix, and perform checks.
-
- Args:
- str_prefix: The string prefix for the ID type (e.g., "MID").
- Must be unique across all ID types.
- allow_custom_name: If True, bypasses the check that the class name
- must end with "Id". Defaults to False.
- **kwargs: Forwarded to the parent's __init_subclass__.
-
- Raises:
- ValueError: If the `str_prefix` is already registered by another
- ID type.
- TypeError: If `allow_custom_name` is False and the class name
- does not end with "Id".
- """
- super().__init_subclass__(**kwargs)
-
- if str_prefix in BaseId._registered_prefixes:
- raise ValueError(
- f"Prefix '{str_prefix}' is already registered. "
- "ID prefixes must be unique."
- )
- BaseId._registered_prefixes.add(str_prefix)
-
- if not allow_custom_name and not cls.__name__.endswith("Id"):
- raise TypeError(
- f"Class name '{cls.__name__}' for an ID class must end with 'Id' "
- "(e.g., 'SomeId'), or use `allow_custom_name=True`."
- )
-
- cls._str_prefix = str_prefix
-
- def __init__(self, id_: int, /) -> None:
- """Initialize this instance.
-
- Args:
- id_: The numeric unique identifier.
-
- Raises:
- ValueError: If the ID is negative.
- """
- if id_ < 0:
- raise ValueError(f"{type(self).__name__} can't be negative.")
- self._id = id_
-
- @property
- def str_prefix(self) -> str:
- """The prefix used for the string representation of this ID."""
- return self._str_prefix
-
- def __int__(self) -> int:
- """Return the numeric ID of this instance."""
- return self._id
-
- def __eq__(self, other: object) -> bool:
- """Check if this instance is equal to another object.
-
- Equality is defined as being of the exact same type and having the same
- underlying ID.
- """
- # pylint thinks this is not an unidiomatic typecheck, but in this case
- # it is not. isinstance() returns True for subclasses, which is not
- # what we want here, as different ID types should never be equal.
- # pylint: disable-next=unidiomatic-typecheck
- if type(other) is not type(self):
- return NotImplemented
- # We already checked type(other) is type(self), but mypy doesn't
- # understand that, so we need to cast it to Self.
- other_id = cast(Self, other)
- return self._id == other_id._id
-
- def __lt__(self, other: object) -> bool:
- """Check if this instance is less than another object.
-
- Comparison is only defined between instances of the exact same type.
- """
- # pylint: disable-next=unidiomatic-typecheck
- if type(other) is not type(self):
- return NotImplemented
- other_id = cast(Self, other)
- return self._id < other_id._id
-
- def __hash__(self) -> int:
- """Return the hash of this instance.
-
- The hash is based on the exact type and the underlying ID to ensure
- that IDs of different types but with the same numeric value have different hashes.
- """
- return hash((type(self), self._id))
-
- def __repr__(self) -> str:
- """Return the string representation of this instance."""
- return f"{type(self).__name__}({self._id!r})"
-
- def __str__(self) -> str:
- """Return the short string representation of this instance."""
- return f"{type(self)._str_prefix}{self._id}"
-
-
-@final
-class EnterpriseId(BaseId, str_prefix="EID"):
- """A unique identifier for an enterprise account."""
-
-
-@final
-class MicrogridId(BaseId, str_prefix="MID"):
- """A unique identifier for a microgrid."""
-
-
-@final
-class ComponentId(BaseId, str_prefix="CID"):
- """A unique identifier for a microgrid component."""
-
-
-@final
-class SensorId(BaseId, str_prefix="SID"):
- """A unique identifier for a microgrid sensor."""
diff --git a/tests/client_test_cases/get_microgrid_info/defaults_case.py b/tests/client_test_cases/get_microgrid_info/defaults_case.py
index 750ab62..5776eef 100644
--- a/tests/client_test_cases/get_microgrid_info/defaults_case.py
+++ b/tests/client_test_cases/get_microgrid_info/defaults_case.py
@@ -9,10 +9,10 @@
from frequenz.api.common.v1.microgrid import microgrid_pb2 as microgrid_common_pb2
from frequenz.api.microgrid.v1 import microgrid_pb2
+from frequenz.client.common.microgrid import EnterpriseId, MicrogridId
from google.protobuf.empty_pb2 import Empty
from frequenz.client.microgrid import MicrogridInfo, MicrogridStatus
-from frequenz.client.microgrid.id import EnterpriseId, MicrogridId
# No client_args or client_kwargs needed for this call
diff --git a/tests/client_test_cases/get_microgrid_info/full_case.py b/tests/client_test_cases/get_microgrid_info/full_case.py
index 437b4e2..4f1e9f9 100644
--- a/tests/client_test_cases/get_microgrid_info/full_case.py
+++ b/tests/client_test_cases/get_microgrid_info/full_case.py
@@ -13,6 +13,7 @@
from frequenz.api.common.v1.microgrid import microgrid_pb2 as microgrid_common_pb2
from frequenz.api.microgrid.v1 import microgrid_pb2
from frequenz.client.base.conversion import to_timestamp
+from frequenz.client.common.microgrid import EnterpriseId, MicrogridId
from google.protobuf.empty_pb2 import Empty
from frequenz.client.microgrid import (
@@ -22,7 +23,6 @@
MicrogridInfo,
MicrogridStatus,
)
-from frequenz.client.microgrid.id import EnterpriseId, MicrogridId
# No client_args or client_kwargs needed for this call
diff --git a/tests/test_id.py b/tests/test_id.py
index d401d2a..d340413 100644
--- a/tests/test_id.py
+++ b/tests/test_id.py
@@ -1,18 +1,14 @@
# License: MIT
# Copyright © 2025 Frequenz Energy-as-a-Service GmbH
-"""Tests for IDs."""
+"""Tests for the microgrid and component IDs."""
from dataclasses import dataclass
import pytest
-
-from frequenz.client.microgrid.id import (
- ComponentId,
- EnterpriseId,
- MicrogridId,
- SensorId,
-)
+from frequenz.client.common.microgrid import MicrogridId
+from frequenz.client.common.microgrid.components import ComponentId
+from frequenz.client.common.microgrid.sensors import SensorId
@dataclass(frozen=True)
@@ -25,7 +21,6 @@ class IdTypeInfo
# Define all ID types to test here
ID_TYPES: list[IdTypeInfo] = [
- IdTypeInfo(EnterpriseId, "EID"),
IdTypeInfo(MicrogridId, "MID"),
IdTypeInfo(ComponentId, "CID"),
IdTypeInfo(SensorId, "SID"),
diff --git a/tests/test_microgrid_info.py b/tests/test_microgrid_info.py
index f170175..8b9a90f 100644
--- a/tests/test_microgrid_info.py
+++ b/tests/test_microgrid_info.py
@@ -10,6 +10,7 @@
import pytest
from frequenz.api.common.v1.grid import delivery_area_pb2
from frequenz.api.common.v1.microgrid import microgrid_pb2
+from frequenz.client.common.microgrid import EnterpriseId, MicrogridId
from frequenz.client.microgrid import (
DeliveryArea,
@@ -19,7 +20,6 @@
MicrogridStatus,
)
from frequenz.client.microgrid._microgrid_info_proto import microgrid_info_from_proto
-from frequenz.client.microgrid.id import EnterpriseId, MicrogridId
@dataclass(frozen=True, kw_only=True)
diff --git a/tests/util.py b/tests/util.py
index 7eb619a..05c42d0 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -1214,7 +1214,13 @@ def __exit__
"""Exit the context manager."""
self._patched_client_class.__exit__(*args, **kwargs)
- def _fake_connect(self, client: ClientT, server_url: str | None = None) -> None:
+ def _fake_connect(
+ self,
+ client: ClientT,
+ server_url: str | None = None,
+ auth_key: str | None = None, # pylint: disable=unused-argument
+ sign_secret: str | None = None, # pylint: disable=unused-argument
+ ) -> None:
"""Fake connect method that does nothing."""
# pylint: disable=protected-access
if server_url is not None and server_url != client._server_url: # URL changed |
|
Oh, and I created a v0.17.x branch (based on the current v0.x.x) and use it as the base branch for this PR. Since lately we needed to make some (breaking) changes to the current v0.x.x. (v0.15), and we are going to do the update to v0.17 as a series of PRs, I though it was a good idea to leave some room in case we need some more releases before the upgrade. Also it makes it easier to match which API version the client is using, as it is getting a bit confusing. |
This is a major breaking change, as we jump to the API specification version 0.17.x, which introduces big and fundamental breaking changes. This also starts using the
v1namespace infrequenz-api-common, which also introduces major breaking changes. It would be very hard to detail all the API changes here, please refer to the Microgrid API releases and Common API releases.This PR also only focus on migrating the current functionality to v0.17.x, and it is the starting point, only implementing
GetMicrogridMetadata. Other RPCs will be implemented in follow-up PRs.Also many of the things added here will be moved to other repos. All protobuf wrappers for
api-commonwill be moved toclient-common, and the test framework for API clients will be moved toclient-base.Part of #55.